Fix logout redirect to landing page#84
Conversation
|
@jordansilly77-stack is attempting to deploy a commit to the Shiv Raj Singh's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughNavbar.jsx's logout handler was modified to import toast from react-toastify, add setIsLoggedin to the AppContent context destructuring, and change post-logout behavior to clear state and toast-notify with navigation to "/" on success, or show an error toast on failure instead of redirecting. ChangesLogout flow fix
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Navbar
participant API
participant Toast
User->>Navbar: Click Logout
Navbar->>API: Call logout endpoint
alt Success
API-->>Navbar: 200 OK
Navbar->>Navbar: setIsLoggedin(false), clear userData, clear localStorage
Navbar->>Toast: Show success toast
Navbar->>Navbar: Navigate to "/"
else Failure
API-->>Navbar: Error response
Navbar->>Toast: Show error toast
end
Related Issues: Suggested labels: bug, frontend Suggested reviewers: imuniqueshiv 🐰 A logout that once led astray, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Local validation passed with:\n\n- npm run lint --prefix client\n- npm run build --prefix client\n\nThe Vercel preview check currently points to a Vercel authorization URL, so it looks like a preview deployment authorization issue rather than a client build failure. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
client/src/components/Navbar.jsx (1)
135-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate logout through
AppContext.logoutUser
handleLogoutduplicates the shared auth reset and bypassesisLoggingOut, so this flow can drift. HavelogoutUserreturn success/failure (or throw), then keep only the menu-closing andnavigate("/")steps inNavbar.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/components/Navbar.jsx` around lines 135 - 155, The logout flow in handleLogout duplicates auth reset logic instead of using the shared AppContext.logoutUser path, which can drift and ignores isLoggingOut. Update AppContext.logoutUser to return success/failure or throw after doing the auth reset, then change Navbar.handleLogout to call logoutUser and keep only the UI cleanup steps like closing menus, showing the toast, and navigating to "/".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@client/src/components/Navbar.jsx`:
- Around line 135-155: The logout flow in handleLogout duplicates auth reset
logic instead of using the shared AppContext.logoutUser path, which can drift
and ignores isLoggingOut. Update AppContext.logoutUser to return success/failure
or throw after doing the auth reset, then change Navbar.handleLogout to call
logoutUser and keep only the UI cleanup steps like closing menus, showing the
toast, and navigating to "/".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8189478-68d6-47ac-b61f-de2098a5ca08
📒 Files selected for processing (1)
client/src/components/Navbar.jsx
|
@jordansilly77-stack Thank you for your contribution and for fixing the logout redirect issue! 🚀 The implementation looks good and aligns well with the scope of Issue #83. Before I merge this PR, could you please comment Once you've claimed the issue, I'll be happy to merge your PR. Thanks again for your contribution! |
imuniqueshiv
added
bug
|
@jordansilly77-stack Thank you for your contribution! 🚀 Your implementation looks good and successfully addresses the logout redirect issue. I really appreciate the time and effort you put into this fix. However, this issue had already been claimed by another contributor according to our repository's contribution workflow. To keep the process fair and consistent for everyone, I'll be moving forward with the PR from the contributor who claimed the issue first. Thank you again for your contribution, and I hope you'll continue contributing to MeetOnMemory. There are many other open issues where your help would be greatly appreciated. Happy coding! 💙 |
2 participants
Summary\n- Clear logged-in state after a successful logout\n- Redirect users to the landing page instead of forcing a hard navigation to /login\n- Keep users on the current page and show an error toast if logout fails\n\n## Validation\n- npm run lint --prefix client\n- npm run build --prefix client\n\nCloses #83
Summary by CodeRabbit